-
Notifications
You must be signed in to change notification settings - Fork 623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SOLR-17263: HttpJdkSolrClient doesn't encode curly braces etc #2433
Conversation
5e81b74
to
4443111
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you plan to merge this PR and then I can close out #2435? Your PR looks like a comprehensive fix!
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
Outdated
Show resolved
Hide resolved
solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
Outdated
Show resolved
Hide resolved
754635a
to
6c6e183
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bug fix looks fine to me, all it needs is a CHANGES.txt entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Get this in for 9.6.1 ?
(cherry picked from commit 4c439d0)
(cherry picked from commit 4c439d0)
https://issues.apache.org/jira/browse/SOLR-17263
Description
HttpJdkSolrClient doesn't encode curly braces etc
NB see also #2454
Solution
Use SolrParams' toQueryString() method rather than implicitly using toString() which doesn't encode all chars.
Tests
I've updated tests to include a curly brace character which should be encoded to
%7B
, and a length check on the request body content length for PUT/POST requests. Each subsequent commit resolves some of the test failures.Checklist
Please review the following and check all that apply:
main
branch../gradlew check
.